Skip to content

node: pass no options to EventEmitter from OutgoingMessage and ChildProcess - #47434

Closed
robobun wants to merge 3 commits into
mainfrom
robobun/df5203eb/outgoing-message-ignores-capture-rejections
Closed

robobun wants to merge 3 commits into
mainfrom
robobun/df5203eb/outgoing-message-ignores-capture-rejections

Conversation

@robobun

@robobun robobun commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • new http.ServerResponse(req, { captureRejections: true }) captures rejections in bun run. Node v26.3.0 ignores the option. A non-boolean value throws in Bun only.
  • OutgoingMessage calls Stream.$call(this, options) (src/js/node/_http_outgoing.ts:139). Node calls Stream.call(this). ChildProcess has no constructor, so its arguments reach EventEmitter too.
  • No user reported it. A review of #47381 found that --aot and bun run disagree.

Fix

  • OutgoingMessage calls Stream with undefined, in src/js/node/ and in src/js/ahead-of-time/. ChildProcess gets constructor() { super(); }.
  • Correct because node hands EventEmitter nothing in both places. EventEmitter.captureRejections still works.
  • Verified: new tests in test/js/node/http/node-http-stores-of-the-response-constructor.test.ts and test/js/node/child_process/child_process-node.test.js fail on main. CI runs the new program under --aot.
  • Self-reviewed: 20 concerns raised, 17 addressed. Not done: a 91-row table in place of the program, a counted // Todo: program, and tracking issues.

Background

  • captureRejections is an EventEmitter constructor option. With it, emit() handles a listener's rejected promise.
  • src/js/ahead-of-time/ holds the text that bun build --compile --aot compiles in place of src/js/node/. Both must behave the same.
  • No other place fits: ServerResponse must pass its options on, and so must Stream, as in node.

Downsides

  • A program that relied on the option now ends with an unhandled rejection, as in node. EventEmitter.call(res, { captureRejections: true }) restores capture.
  • Under --aot on x64, new http.OutgoingMessage() in a loop costs 1,056 instructions, against 670. Notes has the cause.
  • Not slower: bun run, new ServerResponse(), a sent ClientRequest, 40 --aot server routes.
Notes

Order. This is a draft because it conflicts with #46528 and #46522 in one line: the SHA-256 of the first text in the head of src/js/ahead-of-time/node/_http_outgoing.ts. The other hunks merge without a conflict. Both branches still have Stream.$call(this, options) (#46528 at 001958b: :145 and :295). On #46528 the rule for such a PR is: it stays a draft and is cut again on top.

Where it comes from. The review comment found that the two texts differ for new ServerResponse(req, { captureRejections: true }). It proposed that the second text read the option. Node does not read it, so this PR goes the other way. No issue mentions captureRejections, and node documents no options for these constructors. Stream.$call(this, options) came with #17093, where Writable.$call(this, options) became a call of Stream.

A program that relied on the option:

import http from "node:http";
const req = { method: "GET", httpVersionMajor: 1, httpVersionMinor: 1, headers: {} };
const res = new http.ServerResponse(req, { captureRejections: true });
res.on("x", async () => { throw new Error("boom"); });
res.emit("x");
setImmediate(() => console.log("destroyed:", res.destroyed));
result
node v26.3.0 Error: boom, exit code 1
main f7607a3, bun run destroyed: true, exit code 0
this PR, bun run error: boom, exit code 1

With EventEmitter.call(res, { captureRejections: true }) after the constructor, node, main and this PR all print destroyed: true, under bun run and under --aot.

The new program, test/aot/parallel/library-modules/http-messages-hand-no-options-to-the-emitter.mjs. It makes a message in five ways. For each it prints three answers: with captureRejections: true, with captureRejections: "yes", and with no options while EventEmitter.captureRejections is true.

new OutgoingMessage, a class that extends it, a class that extends ServerResponse, ServerResponse.call(object) new ServerResponse
node v26.3.0 false, false, true false, false, true
main, bun run true, ERR_INVALID_ARG_TYPE, true true, ERR_INVALID_ARG_TYPE, true
main, --aot true, ERR_INVALID_ARG_TYPE, true false, false, true
this PR, bun run and --aot false, false, true false, false, true

test/aot/run-parallel.ts on the program with a release build: main fails ("it does not exit and print as plain Bun does"), this PR passes.

ChildProcess. Node has function ChildProcess() { FunctionPrototypeCall(EventEmitter, this); (child_process.js:255). For new ChildProcess(options) and for a class that extends it, node prints false, false, true and main prints true, ERR_INVALID_ARG_TYPE, true. child_process.ts has no second text. The test is in child_process-node.test.js. Two tests of child_process.test.ts fail on a debug build in my container, with and without this change. One needs more than 1,024 descriptors, and one takes more than 5 s.

Why undefined and not a missing argument. Stream declares one parameter. In the interpreter a call with no argument costs 83 instructions more, an arity fix-up. The second text already wrote Stream.$call(this, undefined).

Measurements. Release builds of f7607a3 and of this PR, on Linux x64. Instructions in user mode per object, from perf_event_open, in a loop that makes the object and drops it. Each program imports one module. The programs of the last two rows of the second table also import node:events, node:http, node:net and node:stream. The interpreter and --aot values are the median of three readings. The JIT values are the range of six readings, and of three for ChildProcess.

bun run interpreter, main this PR JIT, main this PR
new http.OutgoingMessage() 21,171 21,172 447 to 459 449 to 455
new http.ServerResponse(req) 33,778 33,778 995 to 1,003 992 to 997
new http.ServerResponse(req, { highWaterMark: 7 }) 35,075 34,769 1,005 to 1,009 1,001 to 1,011
new (class extends http.ServerResponse {})(req) 36,539 36,540 985 to 989 981 to 985
new ChildProcess() 8,681 8,120 219 to 221 160 to 161

A server passes options for every response, so the third row is the one a request takes.

bun build --compile --aot main this PR
new http.OutgoingMessage() 670 1,056
new http.OutgoingMessage({ highWaterMark: 7 }) 2,475 1,630
new (class extends http.OutgoingMessage {})() 1,001 1,135
new http.ServerResponse(req) 2,499 2,502
new http.ServerResponse(req, { highWaterMark: 7 }) 3,193 3,194
new (class extends http.ServerResponse {})(req) 6,242 6,462
the same with { highWaterMark: 7 } 6,343 6,460
http.ServerResponse.call(this, req) in a constructor function 10,571 10,564
new ChildProcess() 6,032 6,032
new EventEmitter(), new Readable(), new Writable(), new Duplex() 722, 1,005, 2,189, 2,292 722, 1,005, 2,189, 2,291
new net.Socket() 11,240 11,227

Servers under --aot, instructions a request:

  • bun test/aot/perf/compare-builds.ts --servers node,express,fastify,koa: 32 rows, 0 slower, 0 faster.
  • The node server of test/aot/servers/app with createServer({ ServerResponse: class extends ServerResponse {} }, ..): all 8 routes cost less with this PR, by 0.3% to 2.0%. The text route costs 31,205 to 31,239 on main and 30,595 to 30,616 with this PR.
  • A loop of http.get() against a local server calls operationAOTEnsureExtensionGranules 7 times in all. That holds for 200 and for 1,200 requests, before and after.
  • In a server that uses the built-in classes, four compiled functions change size and all get smaller: OutgoingMessage (construct body) 8,922 to 5,788 bytes, EventEmitter 10,935 to 10,904, initializeProxyReceiver 5,120 to 5,092, OutgoingMessage 3,552 to 3,548.
  • The bun binary has 138,270,840 bytes before and after. size reports 38 bytes more text.

The three rows that rise. They rise by a decision of the slot planner. testaot print program.aot --command="plan-slots layouts fill" references=narrow shows it for the layout of OutgoingMessage:

main this PR
the cell size=160 size=144
_events @40, filled=40% out@8, filled=32%
[kShapeMode] @136, filled=55% likely out@34, filled=44%
_onPendingData @120 out@60

The engine puts a field into the cell when its estimated fill is at least half of the creations. That is isLikelyFilled in AOTSlotPlanSteps.h (oven-sh/WebKit 8c6c963e). With a constant undefined in place of options, the estimate for [kShapeMode] falls below one half. Three fields then lie outside the cell. From the 502nd object on, or after a collection, each new http.OutgoingMessage() calls operationAOTEnsureExtensionGranules once.

The numbers move with the rest of the program. The program below makes 2,519 calls for 3,000 objects. Without its last block it is the loop of the table. With the block, which never runs, it makes 3 calls, as main does:

import http from "node:http";
const N = Number(process.argv[2]);
let sink = 0;
for (let i = 0; i < N; i++) sink += new http.OutgoingMessage().outputSize + 1;
if (sink <= 0) console.log("?");
if (process.argv[3] === "never") {
  for (let i = 0; i < N; i++) {
    const message = new http.OutgoingMessage();
    message._events = { __proto__: null };
    sink += message.outputSize;
  }
}

Two more release builds gave the numbers of this one. One kept the call in both arms of the if. The other also left the options interface as it was. I did not bend the second text to win a layout back.

The cost belongs to the layout on x64. A plain loop of 1,000 new http.OutgoingMessage() makes 519 calls with this PR and 1 on main, on x64. On Alpine aarch64 the same loop makes 40 calls or fewer with this PR. I saw that in CI and have no arm64 machine to look closer. An earlier commit of this PR held that count in a program with a // Todo: line. I took the program out again, because one line cannot be right for both architectures.

src/aot/CLAUDE.md asks for a fix or a revert of a row over +2%. Three rows are over it on x64, and I ask for the exception: this change is what makes the two texts agree with each other and with node.

What is left out. I gave captureRejections: true and "yes" to 68 public constructors and factories of node:* modules that make an emitter. I compared each with node v26.3.0. Besides the two classes of this PR, these differ, on main and with this PR:

  • tls.TLSSocket, tls.connect and https.request. TLSSocket hands its whole options object to net.Socket (src/js/node/tls.ts:790). Node hands ten named keys (wrap.js:590). The fix there is a fixed list of keys, which changes more than this option. tls.connect({ captureRejections: true }) captures in Bun today.
  • tty.ReadStream(fd, options) hands on nothing (src/js/node/tty.ts:65). Node hands the options on.
  • The built-in undici module: Agent has no constructor (src/js/thirdparty/undici.ts:269), so new Agent(options) hands its options on.
  • new http.ServerResponse(req, { highWaterMark: 7 }).writableHighWaterMark is 65536, and 7 in node. The option is stored as in node. The getter at src/js/node/_http_server.ts:3961 answers for the socket that it makes.
  • The two texts read the keys of options in new ServerResponse(req, options) in another order, which a Proxy shows.

Within node:http, Agent, Server, IncomingMessage and ClientRequest agree with node.

Also in the diff. interface OutgoingMessageOptions no longer extends EventEmitterOptions: nothing hands these options to an emitter. tsc -p src/js prints the same 315 lines before and after. The SHA-256 in the head of the second text is that of the new first text.

Suites run.

  • bun bd test on the two test files: with the src/ of main 2 tests fail, with this PR none.
  • Release builds of main and of this PR, each file in a process of its own: the 87 test files of test/js/node/http, child_process, events and cluster, the two stream tests that run programs, and EventEmitterAsyncResource.test.ts. This PR: 1,649 pass. One test fails before and after (spawn still works with more than 10240 fds open, EMFILE in my container). Main differs in the two new tests only.
  • 718 files of test/js/node/test/parallel (test-http-*, test-https-*, test-child-process-*, test-cluster-*, test-event-*, *capture-rejection*, *catch-rejections*): 716 exit with 0 before and after. The other two need bun test.
  • test/aot/run-parallel.ts on the 50 programs of test/aot/parallel/node-http and 16 of library-modules: 66 pass.
  • test/internal/source-lints (194 pass), test/internal/second-texts-of-modules.test.ts, test/aot/every-test-runs-in-ci.test.ts, bun fmt --check, bun lint.

no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/child_process/child_process-node.test.js

OutgoingMessage called Stream with its options, so EventEmitter read
captureRejections from the options of a ServerResponse. Node.js calls
Stream.call(this) there. The ahead-of-time text did the same for every
OutgoingMessage but a ServerResponse made with new.

Both texts now call Stream with undefined.
ChildProcess had no constructor, so the default one handed its arguments
to EventEmitter, which read captureRejections from them. Node.js calls
EventEmitter with none.

A counted program records what the change in node:http costs ahead of
time: after 501 messages, each OutgoingMessage made in a loop asks the
run time for room for three fields.
@robobun

robobun commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:05 AM PT - Oct 30th, 2026

❌ @robobun, your commit b9a3394 has 1 failures in Build #135635 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 47434

That installs a local version of the PR into your bun-47434 executable, so you can run:

bun-47434 --bun

@robobun

robobun commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: closed without a merge

No user reported this difference. A maintainer asked to close #48311 for that reason (review), and the same holds here.

How I reproduced it. Release build of main 2596ac5, against node v26.3.0:

  • new http.ServerResponse(req, { captureRejections: true }) captures rejections, and { captureRejections: "yes" } throws ERR_INVALID_ARG_TYPE. Node ignores the option.
  • The same holds for new http.OutgoingMessage(options), for a class that extends either class, and for new ChildProcess(options).
  • Since AOT: front end, test runs and tests for the rewritten compiler #47942, bun run and bun build --compile --aot agree with each other here. Before it, --aot ignored the option in new ServerResponse().

What a later cut needs, if a user reports this or a maintainer wants node's behaviour. The line numbers are those of main 2596ac5.

  • src/js/node/_http_outgoing.ts:140 and src/js/ahead-of-time/node/_http_outgoing.ts:279: call Stream with undefined, as node's Stream.call(this). Then write the new SHA-256 into line 2 of the second text.
  • src/js/ahead-of-time/node/_http_server.ts:3386-3389: remove the read of options.captureRejections. 83fa81f added it so that --aot does what bun run does.
  • test/aot/parallel/a-response-made-by-hand/reads-its-options.mjs: its first line says that the options are "read, checked and honored".
  • src/js/node/child_process.ts:1148: give class ChildProcess a constructor() { super(); }, as node's ChildProcess hands EventEmitter nothing.
  • The branch of this PR has the program and the two tests. It is a cut on f7607a3, before AOT: front end, test runs and tests for the rewritten compiler #47942.

I built that cut on 2596ac5 and did not push it. With it the two new tests pass, and they fail with the src/ of main. 367 --aot programs pass. Under --aot, the interpreter and the JIT, no row of instructions per object is slower by more than two readings of one build differ.

tls.TLSSocket and tls.connect hand captureRejections on too (src/js/node/tls.ts:791), where node does not. This PR did not change them.

The source diff of that cut
diff --git a/src/js/ahead-of-time/node/_http_outgoing.ts b/src/js/ahead-of-time/node/_http_outgoing.ts
index 75c54845f9c..67cf071dee6 100644
--- a/src/js/ahead-of-time/node/_http_outgoing.ts
+++ b/src/js/ahead-of-time/node/_http_outgoing.ts
@@ -1,5 +1,5 @@
 // What the compiler reads in place of src/js/node/_http_outgoing.ts, which `bun run` evaluates. The two do the same.
-// Written against that file with the SHA-256 7eb68379dcc85e37da343c90d1da52f21b0bd91937cea041c06b56a6db7551fc.
+// Written against that file with the SHA-256 fd216fcabdbf577569fbb632c6d624d1d274693f1f402f11ed8cfdce28f6be12.
 // When that file changes, bring the change here and write the new hash: test/internal/second-texts-of-modules.test.ts.
 
 // Hardcoded module "node:_http_outgoing"
@@ -152,7 +152,7 @@ function isContentDispositionField(s: string) {
 
 declare global {
   namespace $Node {
-    interface OutgoingMessageOptions extends EventEmitterOptions {
+    interface OutgoingMessageOptions {
       highWaterMark?: number;
       rejectNonStandardBodyWrites?: boolean;
     }
@@ -276,7 +276,7 @@ function OutgoingMessage(this: $Node.OutgoingMessage, options?: $Node.OutgoingMe
   // A call on a response that exists lays its fields anew. What its server keeps of the dispatch is not among them, and stays.
   const kept = this.$exchange;
   if (kept !== undefined) http.responseRoad.spillStateReachedThrough!(this);
-  Stream.$call(this, options);
+  Stream.$call(this, undefined);
 
   // Queue that holds all currently pending data, until the response will be
   // assigned to the socket (until it will its turn in the HTTP pipeline).
diff --git a/src/js/ahead-of-time/node/_http_server.ts b/src/js/ahead-of-time/node/_http_server.ts
index aa946e2a188..eb995640350 100644
--- a/src/js/ahead-of-time/node/_http_server.ts
+++ b/src/js/ahead-of-time/node/_http_server.ts
@@ -3383,11 +3383,7 @@ function ServerResponse(
   this._eventsCount = 0;
   this[kShapeMode] = false;
   this._maxListeners ??= undefined;
-  if (options?.captureRejections) {
-    validateBoolean(options.captureRejections, "options.captureRejections");
-    this[kCapture] = !!options.captureRejections;
-    storeEmitWithRejectionCapture(this);
-  } else if ((this[kCapture] = EventEmitterPrototype[kCapture])) {
+  if ((this[kCapture] = EventEmitterPrototype[kCapture])) {
     storeEmitWithRejectionCapture(this);
   }
   this.outputData = [];
diff --git a/src/js/node/_http_outgoing.ts b/src/js/node/_http_outgoing.ts
index ef8cc79cc01..abf4b718d7c 100644
--- a/src/js/node/_http_outgoing.ts
+++ b/src/js/node/_http_outgoing.ts
@@ -137,7 +137,8 @@ function isContentDispositionField(s) {
 }
 
 function OutgoingMessage(options) {
-  Stream.$call(this, options);
+  // As in node, Stream gets no options. `undefined` and not a missing argument: that call needs no arity fix-up.
+  Stream.$call(this, undefined);
 
   // Queue that holds all currently pending data, until the response will be
   // assigned to the socket (until it will its turn in the HTTP pipeline).
diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts
index 3b0faef5b83..846e79795c9 100644
--- a/src/js/node/child_process.ts
+++ b/src/js/node/child_process.ts
@@ -1161,6 +1161,11 @@ class ChildProcess extends EventEmitter {
   channel;
   killed = false;
 
+  // Not the default constructor, which hands its arguments to EventEmitter. Node's takes none.
+  constructor() {
+    super();
+  }
+
   [Symbol.dispose]() {
     if (!this.killed) {
       this.kill();

Its count holds on x64 only. On Alpine aarch64 the same program stays
under the bound, so its Todo line fails there.
@robobun

robobun commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this without a merge.

No user reported this difference. A maintainer asked to close #48311 for that reason (review), and the same holds here.

The disagreement between bun run and --aot that this PR set out from is also gone. Since #47942 (83fa81f), both read options.captureRejections in new ServerResponse().

The status comment says what is left on main. It has the source diff of a cut on 2596ac5, which I did not push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant